Conversation
|
Soooooooooooooo awesome! |
|
Cool!! :) |
|
Good job! |
c03df74 to
6d7fa3f
Compare
|
Updated and rebased to latest master and included the changes made on these PRs:
All the tests of my PR passes but the tests for the |
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
1 similar comment
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
kapunahelewong
left a comment
There was a problem hiding this comment.
Thanks for the changes. It's looking great. I've made a few comments.
|
|
||
| .l-sub-section | ||
| :marked | ||
| You can think about it as a replacement/enhancement for REST (but you can use them both alongside each other) |
|
|
||
| Apollo's `mutate` function returns the result as an Observable. | ||
| The Observable returns a `mutationResult` variable that is structured | ||
| like the `ApolloQueryResult` Typescript Type, where the generic `T` type is a `Hero` type: |
There was a problem hiding this comment.
Typescript Type -> TypeScript type
Please change globally from Typescript to TypeScript.
| networkStatus: NetworkStatus; | ||
| }; | ||
| :marked | ||
| and that's also how you reference that result variable in Typescript. |
There was a problem hiding this comment.
Start a new sentence. How about:
If this looks familiar, it's because that's also how you reference the _____ variable in TypeScript.
Please be sure to specify the variable (sometimes a word like "that" can be vague).
| }; | ||
| :marked | ||
| and that's also how you reference that result variable in Typescript. | ||
| So in order the get the actual hero data being returned, you access those values like so: |
There was a problem hiding this comment.
How about (or something like - we just need to streamline it and state exactly what we are doing):
To access the hero data the ______ returns, use dot notation to traverse the
mutationResultand assign it to a _____.
I don't know the exact words that should go in the blanks.
| So in order the get the actual hero data being returned, you access those values like so: | ||
| +makeExample('heroes-graphql/ts/src/app/heroes.component.ts', 'access-mutation-result', 'heroes.component.ts') | ||
| :marked | ||
| and then push them into the `heroes` array. |
There was a problem hiding this comment.
Once you have access to the data, push them into the
heroesarray.
We want a complete sentence here.
| :marked | ||
| Now create a new `networkInterface` class and call it `InBrowserNetworkInterface`. | ||
|
|
||
| That class will have a `schema` property which it will initialize in the constructor. |
There was a problem hiding this comment.
That
This class initializes a
schemaproperty in the constructor.
| That class will have a `schema` property which it will initialize in the constructor. | ||
|
|
||
| Then create a `query` function that will recieve a query as a variable and execute that query using | ||
| `graphql` execute function against the schema property. |
There was a problem hiding this comment.
How about this:
Next, the
queryfunction takes as an argument the request, or query, and executes that query using the GraphQLexecutefunction against the schema property.
There was a problem hiding this comment.
Took almost as is but changed a small thing. let me know if that makes sense
| `graphql` execute function against the schema property. | ||
|
|
||
| You send empty objects to the `rootValue` and `contextValue` arguments of the function and send the | ||
| `variables` and `operationName` arguments that are related to the query request. |
There was a problem hiding this comment.
This:
You send empty objects to the
rootValueandcontextValuearguments of the function
Loses me. I get the second part of the sentence about variables and operationName but where is the first part happening in the code? It may be as simple as saying:
You send empty objects to the
rootValueandcontextValuearguments of the function with X and Y respectively.
X and Y being code snippets like {} or whatever is actually doing it.
There was a problem hiding this comment.
ohh I thought that empty objects are enough.
Ok changed the whole sentence, let me know what you think
| You send empty objects to the `rootValue` and `contextValue` arguments of the function and send the | ||
| `variables` and `operationName` arguments that are related to the query request. | ||
|
|
||
| Now export the new `InBrowserNetworkInterface` class in order to import it to Apollo Client. |
There was a problem hiding this comment.
Now -> Lastly
This lets them know we are at the end of covering what's happening in this code snippet.
| :marked | ||
| Uri: Let's add a conclusion showing what was accomplished in the doc. | ||
| Maybe something like (please add anything I've missed): | ||
| :Kapunahele - done, but not sure if its enough? |
There was a problem hiding this comment.
Let's save the details about the conclusion for when we have finalized everything. Then we can make sure it has everything in the same way we are planning for the table of contents.
9f25ae5 to
20b9f66
Compare
| <a id="top"></a> | ||
| :marked | ||
| GraphQL is a network protocol, a query language for your API and a runtime for fulfilling those queries with your existing data. | ||
|
|
|
|
||
| .l-sub-section | ||
| :marked | ||
| You can think about it as a replacement/enhancement for REST (but you can use them both alongside each other). |
There was a problem hiding this comment.
I've been thinking about this one for a while but only just now came up with this:
GraphQL is a replacement or enhancement for REST and can be used in conjunction with it.
| :marked | ||
| ## What is GraphQL? | ||
|
|
||
| GraphQL is an API query language, helping your Angular app to: |
|
|
||
| GraphQL is an API query language, helping your Angular app to: | ||
|
|
||
| - Fetch exactly the information it needs from your server. |
There was a problem hiding this comment.
your server -> the server
| NoSql, REST, Queues, .NET servers, Java servers or any other technology or data source. | ||
| - GraphQL solves the need of sending multiple requests for multiple resources to the server and | ||
| then running complex joins on the client—without the need to create a custom endpoint like REST does. | ||
| - The GraphQL spec also includes protocols for real-time push updates from the server to the client. |
There was a problem hiding this comment.
Please search for and update spec -> specification and docs -> documentation.
|
|
||
| Apollo's `mutate` function returns the result as an Observable. | ||
| The Observable returns a `mutationResult` variable that is structured | ||
| like the `ApolloQueryResult` TypeScript Type, where the generic `T` type is a `Hero` type: |
| }; | ||
| :marked | ||
| If this looks familiar, it's because that's also how you reference the `mutationResult` variable in TypeScript. | ||
| To access the hero data `mutationResult` returns, use dot notation to traverse `mutationResult` and assign it to a new hero object. |
There was a problem hiding this comment.
object. --> object:
It seems like it would flow better with a semicolon.
| To access the hero data `mutationResult` returns, use dot notation to traverse `mutationResult` and assign it to a new hero object. | ||
| +makeExample('heroes-graphql/ts/src/app/heroes.component.ts', 'access-mutation-result', 'heroes.component.ts') | ||
| :marked | ||
| Once you have created the new object with the data, push it into the `heroes` array. |
There was a problem hiding this comment.
Once --> "Now that..."
| Once you have created the new object with the data, push it into the `heroes` array. | ||
|
|
||
| :marked | ||
| Just like a query, the mutate function returns an Observable you can subscribe to |
There was a problem hiding this comment.
mutate --> mutate
(Please put in code font).
| Just like a query, the mutate function returns an Observable you can subscribe to | ||
| that handles the data you request. | ||
|
|
||
| At this point, your existing heroes app can also add a hero: |
There was a problem hiding this comment.
At this point
also
Now your existing heroes app can add a hero using GraphQL.
01b6e27 to
3ebfe5a
Compare
|
CLAs look good, thanks! |
1 similar comment
|
CLAs look good, thanks! |
99d2a5a to
1d1d352
Compare
kapunahelewong
left a comment
There was a problem hiding this comment.
Was only able to get to line 227. Very simple changes. Will have to return to continue.
| GraphQL is a replacement or enhancement for REST and can be used in conjunction with it. | ||
|
|
||
| :marked | ||
| GraphQL provides a complete and understandable description of the data in your API, gives clients the power to ask for exactly what they need and nothing more, makes it easier to evolve APIs over time, and enables powerful developer tools. |
There was a problem hiding this comment.
Since these first three sentences start with "GraphQL", could we vary the language a little? Maybe one could start with "The GraphQL..." <--- I don't know what the right word would be there. In regular language, interface would work, but considering the context, we can't use that, right? Another way we could reduce repetition in the language could be by combining two of these sentences.
Maybe the first and third could be combined with the third starting with "It" instead. Then we leave the subsection immediately after.
| ## What is GraphQL? | ||
|
|
||
| GraphQL is an API query language, helping your Angular app: | ||
|
|
There was a problem hiding this comment.
, helping your Angular app: --> "that helps your Angular app do the following:"
| - Merge multiple dependencies into one single response from the server. | ||
| - Handle server data dependency in a component structure. | ||
|
|
||
| It’s also important to understand that: |
There was a problem hiding this comment.
that --> "three key points" or "these key points"
We want intros to lists to be complete sentences, rather than having the list items complete the sentence. I just recently came to understand that if you're wondering why I hadn't mentioned it before.
| It’s also important to understand that: | ||
|
|
||
| - **GraphQL is not a data source**. The GraphQL runtime works on top of any data source—SQL, | ||
| NoSql, REST, Queues, .NET servers, Java servers or any other technology or data source. |
|
|
||
| - **GraphQL is not a data source**. The GraphQL runtime works on top of any data source—SQL, | ||
| NoSql, REST, Queues, .NET servers, Java servers or any other technology or data source. | ||
| - GraphQL solves the need of sending multiple requests for multiple resources to the server and |
There was a problem hiding this comment.
Please remove one of the occurrences of "multiple" since they are close together. Does that still work? Is there another word we could use for the second one? Maybe "various"?
|
|
||
| :marked | ||
| The `watchQuery` function tells the Apollo Client what data | ||
| this component needs. The Apollo Client then returns the |
There was a problem hiding this comment.
This is the first mention of the Apollo Client. We should add a phrase explaining in just a few words (to preserve brevity) what the Apollo Client is:
"....the Apollo Client, the x that lets you x, what data...."
or:
"....the Apollo Client, covered later,...."
| necessary data to the component as an Observable. | ||
| For example, adding an `age` field to the app is | ||
| simple because you only change the component (you cover | ||
| this syntax [later](#querying) in the cookbook) And then modify the template accordingly: |
There was a problem hiding this comment.
"And then modify" --> "and then modify".
|
|
||
| :marked | ||
| That might work for simple cases but the problem here is that `getHeroes` might fetch | ||
| more information than the app really needs for each hero. |
There was a problem hiding this comment.
the problem here is that --> but as your app grows
| and migrate gradually from them. | ||
|
|
||
| :marked | ||
| ### Typed API, tooling and auto documentation |
There was a problem hiding this comment.
please put a comma after and
| The full documentation can be found on the [Apollo Client website](http://dev.apollodata.com/). | ||
|
|
||
| :marked | ||
| The starting point for the app is the [Tour of Heroes tutorial](https://github.com/Urigo/quickstart/archive/graphql-start.zip) app at its end state. |
There was a problem hiding this comment.
This is helpful, but we can't rely on third party for the download. We should change this to the live example/downloadable example of the TOH.
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
|
@googlebot I signed it |
|
FYI, I have to switch gears for a little bit, but will be back to work on this PR. Looking forward to seeing this cookbook in the docs! |
Use ISubscription because this from Apollo is not compatible with that from RxJS
Hello!
This PR is for an Angular & GraphQL cookbook and includes everything an Angular developer needs in order to integrate GraphQL into their Angular apps.
It includes:
Full cookbook of how to use GraphQL in your Angular app
Full Clone of Tour Of Heroes last chapter with in-memory-graphql instead of on-memory REST
All Tests pass both for JIT and AOT
Working Plunker
Green Lint
We've also updated a lot of libraries during that process to be compatible with Rollup and AOT
apollo-client
apollo-client-rxjs
apollo-angular (and renaming from angular2-apollo #JustAngular)
GraphQL-js
I believe I’ve followed all the guidelines needed but I’m also here in SF for 2 weeks if you need me to come over for a review!